@sumit-bose, could you please convert corresponding commit messages to release notes?

Review done using Claude Code / claude-opus-4-6

Functional Issues

* **EC JWK missing `alg` field breaks JWT signing with EC keys.** `ec_priv_key_jwk` (`src/util/cert/libcrypto/cert.c:388-395`) emits `kty`, `crv`, `x`, `y`, `d`, and `x5t#S256` but no `"alg"` field. The RSA counterpart `rsa_priv_key_jwk` includes `"alg":"RS256"` (line 602). In `get_jwt()` (`src/oidc_child/oidc_child_json.c:1256-1260`), `alg` is extracted via `get_json_const_string_from_keys(jwk, "alg")` and the function fails with "JWK is missing required attributes" if it is NULL. This means **JWT client assertion with EC keys will always fail**. The EC JWK should include an appropriate `alg` value: `"ES256"` for P-256, `"ES384"` for P-384, or `"ES512"` for P-521.

fixed

* **No OpenSSL < 3.0 compatibility for new crypto functions.** `sss_rsa_get_priv_comps` (`src/util/cert/libcrypto/cert.c:417`) and `sss_ec_get_x_y_d` (line 253) use `EVP_PKEY_get_bn_param()` and `OSSL_PKEY_PARAM_*` macros, which are OpenSSL 3.0+ only. Similarly, `get_cert_sha256_hash` (line 1038) uses `EVP_MD_fetch()` / `EVP_MD_free()`, also 3.0+ APIs. The `<openssl/core_names.h>` include is guarded by `#if OPENSSL_VERSION_NUMBER >= 0x30000000L` (line 24), so these functions will fail to compile on OpenSSL < 3.0. The existing functions in the same file (`sss_rsa_get_key`, `sss_ec_get_key`) have proper dual-path implementations for both pre- and post-3.0. Either add version guards/fallbacks, or guard `get_jwk_from_pkcs12` and its callees with `#if OPENSSL_VERSION_NUMBER >= 0x30000000L`, and provide a stub returning `ENOTSUP` for older versions. (I note the gemini bot claimed these were fixed after "/gemini see update" replies, but the code at the current PR HEAD — commit `57117fd16` — still lacks version guards.)

not fixed yet

* **`client_credentials_grant` does not support JWT/mTLS auth methods.** `client_credentials_grant()` (`src/oidc_child/oidc_child_curl.c:858-906`) always appends `client_secret` directly to POST data (line 881) rather than routing through `append_to_creds_to_post_data()`. When `oidc_get_id()` calls this function with `CAM_JWT`, the PKCS#12 password is incorrectly sent as `client_secret` in the POST body instead of a JWT client assertion. Similarly for `CAM_MTLS` — the `client_secret` (which is actually the PKCS#12 password) should not appear in the POST data at all.

fixed

* **`refresh_token` does not support JWT/mTLS auth methods.** `refresh_token()` (`src/oidc_child/oidc_child_curl.c:939-946`) directly appends `client_secret` to POST data rather than using `append_to_creds_to_post_data()`. Token refresh will fail when using JWT or mTLS client authentication.

fixed

* **`get_jwt_payload` truncates timestamps to `int`.** In `get_jwt_payload()` (`src/oidc_child/oidc_child_json.c:1202-1209`), `time_t` values (`iat`, `nbf`, `exp`) are cast to `(int)` and packed with `json_pack` format `"s:i"`. On 32-bit platforms (or post-Y2038), `time_t` values will overflow `int`. Use `"s:I"` format (which expects `json_int_t`) instead.

fixed

Nits & Non-functional Issues

* **Missing space in error message.** In `check_client_auth()` (`src/oidc_child/oidc_child.c:423-424`), the string literals `"not needed for"` and `"authentication with client secret.\n"` are concatenated without a space, producing: "not needed forauthentication with client secret."

fixed

* **Debug message copy-paste error.** In `sss_ec_get_x_y_d()` (`src/util/cert/libcrypto/cert.c:280`), the debug message for the `d` (private key) parameter says "Failed to retrieve EC y coordinate." — should say "private key" or "d parameter".

fixed

* **Duplicate `JOSE_CFLAGS`/`JOSE_LIBS` in Makefile.am.** `$(JOSE_CFLAGS)` is added to both `SSS_CERT_CFLAGS` and directly to `libsss_cert_la_CFLAGS`. Since `libsss_cert_la_CFLAGS` already includes `$(SSS_CERT_CFLAGS)`, the jose flags appear twice. Same for `$(JOSE_LIBS)` in `libsss_cert_la_LIBADD`.

fixed

* **Dead code in `append_jwt_to_postdata`.** At `src/oidc_child/oidc_child_curl.c:567`, `talloc_free(out)` is called when `out` is already NULL (since `append_to_post_data` returned NULL). The call is harmless but misleading.

fixed

* **Missing `:relnote:` tags.** Commits "oidc_child: add JWT authentication" and "oidc_child: add pkcs12-client-creds option" introduce new user-visible CLI options (`--pkcs12-client-creds`, `--client-auth-method`) and new authentication methods but lack `:relnote:` tags in their commit messages.

not yet added

* **`static char curve_name[4096]` in `sss_ec_get_x_y_d`.** The `curve_name` buffer at `src/util/cert/libcrypto/cert.c:261` is declared `static`, which makes the function non-reentrant. While unlikely to cause issues in the current single-threaded usage, a stack-allocated buffer would be safer.

fixed

Review of Existing Review Comments

* **Gemini: curl `ret`/`res` variable mismatch in `set_http_opts`** — The author responded with "/gemini see update" and gemini confirmed this was fixed. The current code at `src/oidc_child/oidc_child_curl.c:377-397` correctly uses `res` for all `curl_easy_setopt` calls. **Resolved.**

* **Gemini: OpenSSL 3.0+ API usage without version guards in `sss_ec_get_x_y_d`** — The author responded with "/gemini see update" and gemini claimed the fix was applied. However, examining the current PR HEAD (`57117fd16`), `sss_ec_get_x_y_d` at line 253 still uses OpenSSL 3.0+ APIs without `#if` guards. **Not resolved** in the code I reviewed.

* **Gemini: `sss_rsa_get_priv_comps` incomplete for OpenSSL < 3.0** — Similar situation: gemini claimed the fix was applied, but the current code at line 417 still has a single implementation using only 3.0+ APIs. **Not resolved** in the code I reviewed.

* **Gemini: BIGNUM memory leaks in `ec_priv_key_jwk`** — The current code at `src/util/cert/libcrypto/cert.c:409-413` properly frees `x`, `y`, `d` via `BN_free()` and `ec_group` via `EC_GROUP_free()` in the cleanup block. **Resolved.**

* **Gemini: BIGNUM memory leaks in `rsa_priv_key_jwk`** — The current code at lines 626-633 properly frees all eight BIGNUMs. **Resolved.**

* **Gemini: X509 cert memory leak in `get_jwk_from_pkcs12`** — The current code at line 1146 calls `X509_free(cert)` in the `done` block. **Resolved.**

* **CodeQL: Poorly documented functions** — `sss_rsa_get_priv_comps` (115 lines) and `rsa_priv_key_jwk` (113 lines) were flagged. SSSD coding style says "Avoid comments that do not add value to the code" — these functions are straightforward enough that their names describe their purpose. Not a concern per SSSD conventions.

Hi,

I have a question about where to add the code form the first patch, i.e. get_jwk_from_pkcs12() and the new functions it calls.

src/util/cert/libcrypto/cert.c seems quite natural because it already contains get_ssh_key_from_cert() which also creates a different representation of a key from a certificates although here only the public one.

However, due to the requirement to do base64-url encoding the dependency to libjose is added and as a result libjose is now linked into various SSSD components which have no use for it.

As an alternative all the new functions can be move to a (new) file in oidc_child/ so that the libjose dependency can be kept with oidc_child.

What would you prefer?

bye,
Sumit

However, due to the requirement to do base64-url encoding the dependency to libjose is added and as a result libjose is now linked into various SSSD components which have no use for it.

Via libsss_cert.so?

As an alternative all the new functions can be move to a (new) file in oidc_child/ so that the libjose dependency can be kept with oidc_child.

Since oidc_child is the only user so far, this would be fine, imo.